Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue 293 by handling default value with type ATV_BITVECTOR #297

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brchiu
Copy link
Contributor

@brchiu brchiu commented Sep 27, 2018

  1. Modify asn1p_y.c and asn1p_y.y to fix an error that
    function _convert_bitstring2binary() always return NULL.

  2. Add handling for default value with type ATV_BITVECTOR in
    function try_inline_default().

ref comments in #293

@coveralls
Copy link

coveralls commented Sep 27, 2018

Coverage Status

Coverage decreased (-0.1%) to 71.014% when pulling 83c3166 on brchiu:fix_issue_293_incorrest_OER_encoding_of_default_BIT_STRING_value into 4cc779f on vlm:master.

if(expr->marker.default_value == NULL
|| expr->marker.default_value->type != ATV_STRING)
if(expr->marker.default_value) {
if(!(etype & ASN_STRING_KM_MASK) &&

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure it should be && and not ||? The logic seems to need "if the type is not a STRING derivative, or default not provided, or (default is neither BIT STRING nor OCTET STRING), then break". Correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BIT STRING -> etype : ASN_BASIC_BIT_STRING , default_value_type : ATV_BITVECTOR
OCTET STRING -> etype : ASN_BASIC_OCTET_STRING, default_value_type : ATV_BITVECTOR
IA5String -> etype : ASN_STRING_IA5String, default_value_type : ATV_STRING

However, ASN_BASIC_BIT_STRING and ASN_BASIC_OCTET_STRING do not have bits set with ASN_STRING_KM_MASK.

I admit that this is a little obscure but I tried to be safe for possible combinations I could figure out.
To simplify it, perhaps we could just check default_value->type against ATV_STRING and ATV_BITVECTOR.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Would you say that ATV_STRING and ATV_BITSTRING cover all the possibilities we want to include here?

1. Modify asn1p_y.c and asn1p_y.y to fix an error that
   function _convert_bitstring2binary() always return NULL.

2. Add handling for default value with type ATV_BITVECTOR in
   function try_inline_default().
@brchiu brchiu force-pushed the fix_issue_293_incorrest_OER_encoding_of_default_BIT_STRING_value branch from da56723 to 83c3166 Compare September 30, 2018 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants